Skip to content

fix(integrations): update usage of is_enabled data forwarding #103584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Nov 20, 2025
Merged

fix(integrations): update usage of is_enabled data forwarding #103584

merged 5 commits into from
Nov 20, 2025
+24 −26

Conversation

@liuirene256
Copy link
Member

@liuirene256 liuirene256 commented Nov 19, 2025 •
edited
Loading

ECO 1200 and ECO 1269

@liuirene256 liuirene256 requested review from a team as code owners November 19, 2025 00:16
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 19, 2025
@codecov
Copy link

codecov bot commented Nov 19, 2025 •
edited
Loading

❌ 2 Tests Failed:

Tests completed Failed Passed Skipped
29884 2 29882 245
View the top 2 failed test(s) by shortest run time 
tests.sentry.integrations.api.serializers.rest_framework.test_data_forwarder.DataForwarderSerializerTest::test_required_fields
Stack Traces | 0.977s run time 
#x1B[1m#x1B[.../serializers/rest_framework/test_data_forwarder.py#x1B[0m:80: in test_required_fields
    assert not serializer.is_valid()
#x1B[1m#x1B[31mE   AssertionError: assert not True#x1B[0m
#x1B[1m#x1B[31mE    +  where True = <bound method BaseSerializer.is_valid of DataForwarderSerializer(data={'organization_id': 4557141661450240, 'provider'...s 'dict'>)\n    project_ids = ListField(allow_empty=True, child=IntegerField(), default=<class 'list'>, required=False)>()#x1B[0m
#x1B[1m#x1B[31mE    +    where <bound method BaseSerializer.is_valid of DataForwarderSerializer(data={'organization_id': 4557141661450240, 'provider'...s 'dict'>)\n    project_ids = ListField(allow_empty=True, child=IntegerField(), default=<class 'list'>, required=False)> = DataForwarderSerializer(data={'organization_id': 4557141661450240, 'provider': <DataForwarderProviderSlug.SEGMENT: 'se...ss 'dict'>)\n    project_ids = ListField(allow_empty=True, child=IntegerField(), default=<class 'list'>, required=False).is_valid#x1B[0m
tests.sentry.integrations.api.endpoints.test_data_forwarding.DataForwardingIndexPostTest::test_create_without_project_ids
Stack Traces | 1.39s run time 
#x1B[1m#x1B[.../api/endpoints/test_data_forwarding.py#x1B[0m:359: in test_create_without_project_ids
    assert data_forwarder.project.count() == 0
#x1B[1m#x1B[31mE   AttributeError: 'DataForwarder' object has no attribute 'project'. Did you mean: 'projects'?#x1B[0m

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

leeandher
leeandher approved these changes Nov 19, 2025
View reviewed changes
Copy link
Member

@leeandher leeandher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, may just want a test for the new models to prevent a regression

cursor[bot]
cursor bot reviewed Nov 19, 2025
View reviewed changes
cursor[bot]
cursor bot reviewed Nov 20, 2025
View reviewed changes
src/sentry/integrations/api/serializers/rest_framework/data_forwarder.py
)
for project_id in project_ids
]
)
Copy link
Contributor

@cursor cursor bot Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Update without project_ids deletes all enrolled projects

The update method always processes project_ids from validated_data, but this field now has a default value of an empty list when not provided in the request. When updating a data forwarder's main config without explicitly providing project_ids, the serializer will use the default empty list, causing the code to delete all enrolled project associations. The method should only modify project_ids if they were actually present in the request data.

Fix in Cursor Fix in Web

@liuirene256 liuirene256 merged commit 8456c24 into master Nov 20, 2025
65 checks passed
@liuirene256 liuirene256 deleted the irene/fixxx branch November 20, 2025 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@leeandher leeandher leeandher approved these changes

Assignees

No one assigned

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@liuirene256 @leeandher